feat(exex): subscribe to notifications with head using `ExExContext` #11500

shekhirin · 2024-10-04T20:41:46Z

Similar to #11495, but takes an approach of a newtype wrapper with a private enum, so that we don't expose the internal Invalid variant to the user.

mattsse

this makes sense, only have naming nits.

this makes the stream API worse for regular streams, but I think we can live with that.

perhalps we could combine this with parts of #11495 and replace Option<ExexInner> with the enum?

https://github.com/paradigmxyz/reth/pull/11495/files#diff-ffb8568607451c18b9e789d3626ae3379862cb33fc72b386e5419a79fa9dbe5fR29

mattsse · 2024-10-07T10:43:29Z

crates/exex/exex/src/context.rs

+    /// Replaces notifications stream with a stream of notifications without a head.
+    ///
+    /// Consumes the type and returns a new instance of [`ExExContext`].
+    pub fn with_notifications_without_head(self) -> Self {


isn't this the default?

Can we come up with a less confusing name? :)

yep, removed these methods on the ExExContext and just left the mutating versions

mattsse · 2024-10-07T10:44:55Z

crates/exex/exex/src/notifications.rs

-}
+/// A stream of [`ExExNotification`]s.
+#[derive(Debug)]
+pub struct ExExNotifications<P, E>(Option<ExExNotificationsInner<P, E>>);


please convert this to a field

mattsse · 2024-10-07T10:45:02Z

crates/exex/exex/src/notifications.rs

+#[derive(Debug)]
+enum ExExNotificationsInner<P, E> {
+    WithoutHead(ExExNotificationsWithoutHead<P, E>),
+    WithHead(ExExNotificationsWithHead<P, E>),


crates/exex/exex/src/notifications.rs

mattsse · 2024-10-07T10:51:33Z

crates/exex/exex/src/context.rs

+    /// provided head.
+    ///
+    /// Consumes the type and returns a new instance of [`ExExContext`].
+    pub fn with_notifications_with_head(self, head: ExExHead) -> Self {


this is a bit verbose, with_with_head

#11500 (comment)

mattsse · 2024-10-07T10:54:37Z

crates/exex/exex/src/notifications.rs

+/// A stream of [`ExExNotification`]s.
+#[derive(Debug)]
+pub struct ExExNotifications<P, E>(Option<ExExNotificationsInner<P, E>>);


there's now no way to obtain the regular ExExNotificationsWithoutHead stream, right?

yes, no way, the fields and the new method are private

mattsse

nits

mattsse · 2024-10-07T14:53:14Z

crates/exex/exex/src/notifications.rs

+        self.inner = ExExNotificationsInner::WithoutHead(match self.inner {
+            ExExNotificationsInner::Invalid => unreachable!(),
+            ExExNotificationsInner::WithoutHead(notifications) => notifications,
+            ExExNotificationsInner::WithHead(notifications) => ExExNotificationsWithoutHead::new(
+                notifications.node_head,
+                notifications.provider,
+                notifications.executor,
+                notifications.notifications,
+                notifications.wal_handle,
+            ),
+        });


isn't this just self.set_without_head()?

mattsse · 2024-10-07T14:53:28Z

crates/exex/exex/src/notifications.rs

+        self.inner = ExExNotificationsInner::WithHead(match self.inner {
+            ExExNotificationsInner::Invalid => unreachable!(),
+            ExExNotificationsInner::WithoutHead(notifications) => ExExNotificationsWithHead::new(
+                notifications.node_head,
+                notifications.provider,
+                notifications.executor,
+                notifications.notifications,
+                notifications.wal_handle,
+                exex_head,
+            ),
+            ExExNotificationsInner::WithHead(notifications) => ExExNotificationsWithHead::new(
+                notifications.node_head,
+                notifications.provider,
+                notifications.executor,
+                notifications.notifications,
+                notifications.wal_handle,
+                exex_head,
+            ),
+        });


same pattern

mattsse · 2024-10-07T14:54:02Z

crates/exex/exex/src/notifications.rs

+        cx: &mut std::task::Context<'_>,
+    ) -> std::task::Poll<Option<Self::Item>> {
+        match &mut self.get_mut().inner {
+            ExExNotificationsInner::Invalid => unreachable!(),


branches are checked in the order they appear, since this is unreachable, this should go last

good point, fixed

* chore(provider): use `get_in_memory_or_storage` on `transactions_by_block_range` (paradigmxyz#11453) * chore(exex): adjust WAL gauge metric names (paradigmxyz#11454) * chore(provider): use `get_in_memory_or_storage_by_block` on `fn block_body_indices` (paradigmxyz#11452) * chore(provider): find `last_database_block_number` with `BlockState` anchor instead (paradigmxyz#11455) * feat: add metrics for failed deliveries (paradigmxyz#11456) * chore: release 1.0.8 (paradigmxyz#11457) * chore(provider): use `block_ref` instead on `BlockState` (paradigmxyz#11458) * feat(rpc): Add codes in execution witness return (paradigmxyz#11443) * fix: ensure the request's gas limit does not exceed the target gas limit (paradigmxyz#11462) * feat(grafana): ExEx WAL (paradigmxyz#11461) * fix: use correct rpc errors (paradigmxyz#11463) * chore(provider): clone after filtering on `sealed_headers_while` (paradigmxyz#11459) Co-authored-by: Matthias Seitz <[email protected]> * Map `TransferKind::EofCreate` => `OperationType::OpEofCreate` (paradigmxyz#11090) Co-authored-by: Matthias Seitz <[email protected]> * fix: windows build (paradigmxyz#11465) * chore: use `block_ref` on `CanonicalInMemoryState` (paradigmxyz#11467) * chore(rpc): remove include_preimage param on debug_execution_witness (paradigmxyz#11466) * feat: make addons stateful (paradigmxyz#11204) Co-authored-by: Arsenii Kulikov <[email protected]> * Relax Trait Bounds on TransactionPool::Transaction and EthPoolTransaction (paradigmxyz#11079) Co-authored-by: Matthias Seitz <[email protected]> * test: add unit tests for `CanonicalChain` (paradigmxyz#11472) * feat(perf): integrate OnStateHook in executor (paradigmxyz#11345) * feat: cleaned up prepare_call_env() (paradigmxyz#11469) Co-authored-by: Matthias Seitz <[email protected]> * chore: use block.body directly (paradigmxyz#11474) * feat: Add metrics to track transactions by type in txpool (paradigmxyz#11403) Co-authored-by: garwah <garwah@garwah> Co-authored-by: Matthias Seitz <[email protected]> * chore: op chainspec (paradigmxyz#11415) * chore(exex): more backfill debug logs (paradigmxyz#11476) * fix(exex): use thresholds in stream backfill (paradigmxyz#11478) * chore(db): capture tx opening backtrace in debug mode (paradigmxyz#11477) * chore(sdk): `SealedHeader` generic over header (paradigmxyz#11429) * chore(lint): fix lint primitives (paradigmxyz#11487) * chore(provider): add more test coverage on `BlockchainProvider::*by_tx_range` queries (paradigmxyz#11480) * test: ensure default hash matches (paradigmxyz#11486) * chore: rm deposit contract config for op (paradigmxyz#11479) * chore(lint): fix lint storage (paradigmxyz#11485) * chore(provider): add more test coverage on `BlockchainProvider::*by_block_range` queries (paradigmxyz#11488) * fix(rpc-eth-types): incorrect error msg(; -> :) (paradigmxyz#11503) Signed-off-by: jsvisa <[email protected]> * Reexport optimism specific crates from `op-reth` (paradigmxyz#11499) * feat: rpc replace function created (paradigmxyz#11501) Co-authored-by: Matthias Seitz <[email protected]> * Add metrics for failed deliveries to Grafana dashboard (paradigmxyz#11481) * chore: Remove duplicate EthereumChainSpecParser in favor of existing EthChainSpecParser (paradigmxyz#11412) Co-authored-by: garwah <garwah@garwah> Co-authored-by: Matthias Seitz <[email protected]> * chore(lint): fix `clippy::needles_lifetimes` (paradigmxyz#11496) * chore: rm from genesis impl (paradigmxyz#11509) * fix: cap gas limit properly (paradigmxyz#11505) * feat: add PoolBuilderConfigOverrides (paradigmxyz#11507) * feat: expose Op node network_config helper (paradigmxyz#11506) * chore(deps): weekly `cargo update` (paradigmxyz#11518) Co-authored-by: github-merge-queue <[email protected]> * test: add unit tests for `PruneLimiter` (paradigmxyz#11517) * feat(provider): add `test_race` to `BlockchainProvider2` tests (paradigmxyz#11523) * fix(tree): make state methods work for historical blocks (paradigmxyz#11265) Co-authored-by: Roman Krasiuk <[email protected]> Co-authored-by: Federico Gimenez <[email protected]> * rpc: use `eth_api()` method (paradigmxyz#11516) * chore: delete rpc-types (paradigmxyz#11528) * feat: add get_highest_tx_by_sender to pools (paradigmxyz#11514) Co-authored-by: Matthias Seitz <[email protected]> * ci: add `windows` cargo check (paradigmxyz#11468) * fix: acquire permit first (paradigmxyz#11537) * chore: dont fail on ttd (paradigmxyz#11539) * grafana: add metrics of all transactions in pool by type (paradigmxyz#11515) Co-authored-by: Emilia Hane <[email protected]> Co-authored-by: Emilia Hane <[email protected]> * feat(exex): subscribe to notifications with head using `ExExContext` (paradigmxyz#11500) * chore: enforce window (paradigmxyz#11540) * Refactor get_payload_bodies_by_hash_with to be non-blocking (paradigmxyz#11511) Co-authored-by: Matthias Seitz <[email protected]> * Introduce Eth PayloadTypes Impl (paradigmxyz#11519) Co-authored-by: Matthias Seitz <[email protected]> * fix(op-reth): add jemalloc feature to optimism-cli for version (paradigmxyz#11543) * fix(grafana): remove rate function from panel "Transactions by Type in Pool" (paradigmxyz#11542) * chore: move ethfiltererror (paradigmxyz#11552) * chore: rm redundant type hint (paradigmxyz#11557) * Introduce Op PayloadTypes Impl (paradigmxyz#11558) Co-authored-by: Matthias Seitz <[email protected]> * chore: chain manual serialisation implementation (paradigmxyz#11538) * chore: rm unused optimism feature (paradigmxyz#11559) * feat(trie): expose storage proofs (paradigmxyz#11550) * chore: rm unused optimism feature from compat (paradigmxyz#11560) * Added InternalBlockExecutionError to execute.rs exports (paradigmxyz#11525) Co-authored-by: Matthias Seitz <[email protected]> * docs(exex): include code for ExEx book from real files (paradigmxyz#11545) * fix: actually configure the custom gas limit (paradigmxyz#11565) * chore: relax trait bound for `EthTransactions` (paradigmxyz#11571) * fix(provider): fix sub overflow on `tx_range` queries for empty blocks (paradigmxyz#11568) * chore(provider): add more test coverage on `BlockchainProvider` non-range queries (paradigmxyz#11564) * feat: adding a new method to network config builder (paradigmxyz#11569) * chore: rm unused optimism feature from engine api (paradigmxyz#11577) * chore: replace some revm deps (paradigmxyz#11579) * chore: rm bad cap function (paradigmxyz#11562) * feat: impl `Encodable2718` and `Decodable2718` for `PooledTransactionsElement` (paradigmxyz#11482) * fix(exex): exhaust backfill job when using a stream (paradigmxyz#11578) * fix: in-memory trie updates pruning (paradigmxyz#11580) Co-authored-by: Matthias Seitz <[email protected]> * chore(providers): test race condition on all `BlockchainProvider2` macro tests (paradigmxyz#11574) * chore: also derive arb for test (paradigmxyz#11588) * chore: bump alloy primitives 0 8 7 (paradigmxyz#11586) * chore(ci): remove expected failures related to checksummed addresses (paradigmxyz#11589) * chore(rpc): use `block_hash` instead on fetching `debug_trace_block` block (paradigmxyz#11587) * feat: add mul support for SubPoolLimit (paradigmxyz#11591) Co-authored-by: Dan Cline <[email protected]> * docs: delete missing part path (paradigmxyz#11590) Co-authored-by: Dan Cline <[email protected]> * fix: simplify reorg handling (paradigmxyz#11592) * fix: use original bytes for codes (paradigmxyz#11593) * perf(rpc): use `Arc<BlockWithSenders` on `full_block_cache` (paradigmxyz#11585) * fix: active inflight count (paradigmxyz#11598) * fix(net): max inflight tx reqs default (paradigmxyz#11602) * fix: set deposit gasprice correctly (paradigmxyz#11603) * fix: set system tx correctly (paradigmxyz#11601) * fix(trie): prefix set extension (paradigmxyz#11605) * feat: add tx propagation mode (paradigmxyz#11594) * feat: add helper function to provde the tx manager config (paradigmxyz#11608) * fix(grafana): set instance variable from `reth_info` metric (paradigmxyz#11607) * fix(net): add concurrency param from config to `TransactionFetcherInfo` (paradigmxyz#11600) Co-authored-by: Matthias Seitz <[email protected]> * perf(rpc): optimistically retrieve block if near the tip on `eth_getLogs` (paradigmxyz#11582) * update fork base commit * remove new book tests --------- Signed-off-by: jsvisa <[email protected]> Co-authored-by: joshieDo <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]> Co-authored-by: Matthias Seitz <[email protected]> Co-authored-by: Francis Li <[email protected]> Co-authored-by: Emilia Hane <[email protected]> Co-authored-by: Arsenii Kulikov <[email protected]> Co-authored-by: Eric Woolsey <[email protected]> Co-authored-by: Thomas Coratger <[email protected]> Co-authored-by: Federico Gimenez <[email protected]> Co-authored-by: Varun Doshi <[email protected]> Co-authored-by: garwah <[email protected]> Co-authored-by: garwah <garwah@garwah> Co-authored-by: greged93 <[email protected]> Co-authored-by: Delweng <[email protected]> Co-authored-by: Parikalp Bhardwaj <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-merge-queue <[email protected]> Co-authored-by: Dan Cline <[email protected]> Co-authored-by: Roman Krasiuk <[email protected]> Co-authored-by: crazykissshout <[email protected]> Co-authored-by: tedison <[email protected]> Co-authored-by: David <[email protected]> Co-authored-by: Emilia Hane <[email protected]> Co-authored-by: Steven <[email protected]> Co-authored-by: Debjit Bhowal <[email protected]> Co-authored-by: Oliver <[email protected]> Co-authored-by: Luca Provini <[email protected]> Co-authored-by: John <[email protected]>

…aradigmxyz#11500)

feat(exex): subscribe to notifications with head using ExExContext

b3ed108

shekhirin added C-enhancement New feature or request A-exex Execution Extensions labels Oct 4, 2024

use option

bb4bcf4

shekhirin force-pushed the alexey/either-exex-notifications branch from b188a4a to bb4bcf4 Compare October 7, 2024 09:18

shekhirin marked this pull request as ready for review October 7, 2024 10:20

shekhirin requested a review from onbjerg as a code owner October 7, 2024 10:20

mattsse requested changes Oct 7, 2024

View reviewed changes

shekhirin force-pushed the alexey/either-exex-notifications branch from 3daad9c to bceafbe Compare October 7, 2024 11:28

replace with enum

7086fcf

shekhirin force-pushed the alexey/either-exex-notifications branch from bceafbe to 7086fcf Compare October 7, 2024 11:31

shekhirin added 2 commits October 7, 2024 12:35

return methods on notifications

94333b0

improve docs

8be28b8

shekhirin requested a review from mattsse October 7, 2024 12:24

mattsse requested changes Oct 7, 2024

View reviewed changes

shekhirin added 2 commits October 7, 2024 15:57

clarify why with_head exists

b3fbb62

nits after review

4ff33ef

shekhirin force-pushed the alexey/either-exex-notifications branch from 3d4dd30 to 4ff33ef Compare October 7, 2024 15:00

mattsse approved these changes Oct 7, 2024

View reviewed changes

shekhirin added this pull request to the merge queue Oct 7, 2024

Merged via the queue into main with commit 8a0bcbb Oct 7, 2024
37 checks passed

shekhirin deleted the alexey/either-exex-notifications branch October 7, 2024 15:44

shekhirin mentioned this pull request Oct 7, 2024

deps: bump Reth and use new notifications API paradigmxyz/reth-exex-examples#28

Merged

vandenbogart pushed a commit to testmachine-ai/reth that referenced this pull request Oct 14, 2024

feat(exex): subscribe to notifications with head using ExExContext (p…

9eb8ffa

…aradigmxyz#11500)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(exex): subscribe to notifications with head using `ExExContext` #11500

feat(exex): subscribe to notifications with head using `ExExContext` #11500

shekhirin commented Oct 4, 2024 •

edited

Loading

mattsse left a comment

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

mattsse Oct 7, 2024

shekhirin Oct 7, 2024 •

edited

Loading

mattsse left a comment

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

mattsse Oct 7, 2024

mattsse Oct 7, 2024

shekhirin Oct 7, 2024

feat(exex): subscribe to notifications with head using ExExContext #11500

feat(exex): subscribe to notifications with head using ExExContext #11500

Conversation

shekhirin commented Oct 4, 2024 • edited Loading

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shekhirin Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat(exex): subscribe to notifications with head using `ExExContext` #11500

feat(exex): subscribe to notifications with head using `ExExContext` #11500

shekhirin commented Oct 4, 2024 •

edited

Loading

shekhirin Oct 7, 2024 •

edited

Loading